fix(follows): cap huge public offsets#357
Conversation
Greptile SummaryThis PR caps oversized
Confidence Score: 4/5Safe to merge; the fix is narrowly scoped to offset parsing and does not touch auth, data writes, or other query logic. The clamping logic is correct and the tests verify both the Supabase range call and the pagination metadata. The main concern is that the capped offset is returned silently in the response — a cursor-driven client has no way to distinguish a legitimately-requested offset of 100,000 from one that was forced there by the cap. The duplicated constant and utility functions across the two route files also introduce a maintenance risk if the values ever need to diverge. Both route files share identical utility functions and the same constant — any future change to the cap or parsing logic must be applied in both places. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Route as followers/following route
participant Parser as parseNonNegativeInt
participant Supabase
Client->>Route: "GET ?offset=999999999&limit=10"
Route->>Parser: parseNonNegativeInt("999999999", 0, 100_000)
Parser-->>Route: 100_000 (clamped)
Route->>Supabase: .range(100000, 100009)
Supabase-->>Route: "{ data: [], count: 0 }"
Route-->>Client: "{ pagination: { offset: 100000, limit: 10, total: 0 } }"
Reviews (1): Last reviewed commit: "fix(follows): cap huge public offsets" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -9,12 +11,12 @@ function parsePositiveInt(value: string | null, fallback: number, max: number): | |||
| return Math.min(parsed, max); | |||
| } | |||
|
|
|||
| function parseNonNegativeInt(value: string | null, fallback: number): number { | |||
| function parseNonNegativeInt(value: string | null, fallback: number, max: number): number { | |||
| const parsed = Number.parseInt(value ?? "", 10); | |||
| if (!Number.isFinite(parsed) || parsed < 0) { | |||
| return fallback; | |||
| } | |||
| return parsed; | |||
| return Math.min(parsed, max); | |||
| } | |||
There was a problem hiding this comment.
MAX_FOLLOW_OFFSET, parsePositiveInt, and parseNonNegativeInt are defined identically in both followers/route.ts and following/route.ts. If the cap value or parsing logic needs to change (e.g., lowering the max, fixing an edge case), both files must be updated in sync — a future maintainer will likely only update one. Extracting these to a shared module (e.g., src/app/api/users/[username]/pagination.ts) would eliminate the drift risk.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const searchParams = request.nextUrl.searchParams; | ||
| const limit = parsePositiveInt(searchParams.get("limit"), 20, 100); | ||
| const offset = parseNonNegativeInt(searchParams.get("offset"), 0); | ||
| const offset = parseNonNegativeInt(searchParams.get("offset"), 0, MAX_FOLLOW_OFFSET); |
There was a problem hiding this comment.
When a client sends offset=999999999, the response returns pagination.offset: 100000 with no indication that the value was clamped. A cursor-driven client that uses the returned offset to drive its next page request (e.g., assumes returned_offset + limit is the next page start) will silently resume from 100000 rather than seeing an error. Consider returning a 400 for offsets above the cap, or at minimum including a field like pagination.offset_clamped: true so callers can detect the truncation.
2 participants
Summary
100_000Fixes #356.
Paid task context: https://ugig.net/gigs/abd6b2a0-e728-48cf-a46f-f99e419ed94e
Verification
.\node_modules\.bin\vitest.CMD run src/app/api/users/[username]/followers/route.test.ts src/app/api/users/[username]/following/route.test.ts.\node_modules\.bin\eslint.CMD src/app/api/users/[username]/followers/route.ts src/app/api/users/[username]/followers/route.test.ts src/app/api/users/[username]/following/route.ts src/app/api/users/[username]/following/route.test.tsgit diff --checkExisting upstream check blockers
.\node_modules\.bin\tsc.CMD --noEmitreaches unrelated existing errors insrc/app/api/affiliates/offers/route.test.ts:53andsrc/app/api/applications/[id]/route.test.ts:21.corepack pnpm run lintreaches unrelated existingreact-hooks/refserrors insrc/hooks/useMessageStream.ts.corepack pnpm run buildcompiles successfully and progresses through static generation when given a process-local dummy OpenAI key, then stops because the local checkout has no Supabase service-role configuration.